-
Notifications
You must be signed in to change notification settings - Fork 163
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
apache camel 3 mixin #826
apache camel 3 mixin #826
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.. A few observations/comments.
apache-camel-mixin/README.md
Outdated
@@ -0,0 +1,121 @@ | |||
THis is a fork of the community dashboard that can be found here: | |||
|
|||
- [Github]([https://link](https://github.com/alainpham/app-archetypes/blob/master/camel-monitoring/dashboards-for-import/apache-camel-micrometer.json)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any changes to the upstream dashboard in this fork?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no there are no changes. I want to keep things in sync. Is there any reccomendation on how to handle this? Should we create a link or something like that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The best choice is to use jsonnet bundler and vendor your existing dashboard, from it's current location..
$ jb init
$ jb install https://github.com/alainpham/app-archetypes/camel-monitoring/dashboards-for-import/
Then you can remove the dashboards
folder, and the copy of the dashboard inside it, and change your mixin.libsonnet
to point to the vendored directory.
{
grafanaDashboards: {
'apache-camel-micrometer.json': (import 'dashboards-for-import/apache-camel-micrometer.json'),
},
}
Then, if you make changes to your repository, you can simply update it with jsonnet bundler. jb update
"schemaVersion": 37, | ||
"style": "dark", | ||
"tags": [], | ||
"templating": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dashboard linter expects that there would be job and instance template variables, which are also used in query matchers. You can read more about that here.
"uid": "${datasource}" | ||
}, | ||
"editorMode": "code", | ||
"expr": "sum(increase(CamelExchangesTotal_total{namespace=\"$namespace\",camelContext=\"$context\"}[$__range]))", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few places where you use rate
, irate
, or increase
with $__range
.
It looks like in each of these cases it is intended to be for "the selected time range" rather than a timeseries showing a rolling interval, for which you should use $__rate_interval
. See target-rate-interval-rule.
Not a problem, just an observation, and it would require a dashboard linter exclusion.
Can we merge this? @alainpham |
If you give me this week I can make the changes suggested by @rgeyer. Thanks |
Definitely! Thanks for looking into it! |
Can we merge this? |
Hi I have added the requested template variables and optimized the dashboard for a kubernetes deployment and eventual log correlation. This can be merged now. |
@gloriasc there has been some changes in the naming convention of the Apache Camel prometheus exporter. I have updated the dashboard to take it into account in the latest commit. Could that be pushed to review? |
I'll add it to our board! |
@alainpham This still LGTM. I can't see what changed with the naming conventions tho, there is no follow-up commit. I'd be comfortable merging this as is (should have done so in May 🤦♂️), but want to make sure that those changes are accounted for. Can you point out what has changed with the exporter, and the related changes here? |
@rgeyer |
Looks good! I'm going to go ahead and merge this. Thank you! |
This PR is to add Apache Camel 3 dashboards as a Grafana Integration.
The dashboard is based on metrics generated by the Micrometer Prometheus instrumentation that is standard across all flavors of Apache Camel (Spring Boot, Quarkus, Camel K)